Modify ObtainAuthToken to use the User model's USERNAME_FIELD and password for authentication instead of assuming username and password.#9674
Conversation
…password' instead of 'username' and 'password' for both the built-in and custom User models
|
If any changes are required, please let me know. |
|
I'm not sure this falls under our current maintenance policy:
One could argue that this improves compatibility with custom Django user models, but on the other hand It's also simple to customise in user-land right now, and this is explained in our docs. If we were to ever accept it, we would need some tests to cover the behaviour with a customised user model... |
|
Thank you for reviewing my pull request and for your feedback. I understand that Django REST Framework is considered feature-complete and that new features are usually only accepted if they align with Django’s ongoing development. However, I see this change as more of a compatibility improvement rather than a new feature. While @browniebroke, Are you saying that it won't be possible to accept or merge this change? |
auvipy
left a comment
There was a problem hiding this comment.
I think this might break existing projects or functionalities
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Pull request overview
This PR modifies the ObtainAuthToken view and AuthTokenSerializer to support custom Django user models that use a different USERNAME_FIELD (e.g., 'email' instead of 'username'). The changes enable dynamic authentication field detection based on the user model's USERNAME_FIELD attribute.
- Dynamically retrieves the
USERNAME_FIELDfrom the active user model instead of hardcoding 'username' - Updates the serializer to create appropriate fields (EmailField or CharField) based on the USERNAME_FIELD type
- Modifies the API schema generation to reflect the correct field name in documentation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| rest_framework/authtoken/serializers.py | Refactored to dynamically create authentication fields based on USERNAME_FIELD and pass credentials to authenticate() |
| rest_framework/authtoken/views.py | Updated schema definition to use USERNAME_FIELD for API documentation |
| docs/api-guide/authentication.md | Added documentation note about custom user model support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| def __init__(self, instance=None, data=None, **kwargs): | ||
| super().__init__(instance, data=data, **kwargs) | ||
| self.identifier_fiend_name = USER_MODEL.USERNAME_FIELD |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| if USER_MODEL.get_email_field_name() == self.identifier_fiend_name: | ||
| self.fields[self.identifier_fiend_name] = serializers.EmailField( |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| write_only=True | ||
| ) | ||
| else: | ||
| self.fields[self.identifier_fiend_name] = serializers.CharField( |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| username=username, password=password) | ||
| if identifier_value and password: | ||
| credentials = { | ||
| self.identifier_fiend_name: identifier_value, |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
|
|
||
| from rest_framework import serializers | ||
|
|
||
| USER_MODEL = get_user_model() |
There was a problem hiding this comment.
The USER_MODEL is evaluated at module import time, which means if the user model changes dynamically or if this module is imported before Django apps are ready, it could reference the wrong model. Consider moving this inside the init method where it's actually used, similar to how it's done in the views.py file.
| def __init__(self, instance=None, data=None, **kwargs): | ||
| super().__init__(instance, data=data, **kwargs) | ||
| self.identifier_fiend_name = USER_MODEL.USERNAME_FIELD | ||
| if USER_MODEL.get_email_field_name() == self.identifier_fiend_name: | ||
| self.fields[self.identifier_fiend_name] = serializers.EmailField( | ||
| label=_(self.identifier_fiend_name.title()), | ||
| write_only=True | ||
| ) | ||
| else: | ||
| self.fields[self.identifier_fiend_name] = serializers.CharField( | ||
| label=_(self.identifier_fiend_name.title()), | ||
| write_only=True | ||
| ) | ||
| self.fields["password"] = serializers.CharField( | ||
| label=_("Password"), | ||
| style={'input_type': 'password'}, | ||
| trim_whitespace=False, | ||
| write_only=True | ||
| ) |
There was a problem hiding this comment.
The new behavior for supporting custom user models with different USERNAME_FIELD values (e.g., 'email') lacks test coverage. Consider adding tests that verify the serializer correctly handles custom user models where USERNAME_FIELD is set to a field other than 'username', such as 'email'.
| USER_MODEL = get_user_model() | ||
| identifier_field_name = USER_MODEL.USERNAME_FIELD |
There was a problem hiding this comment.
The USER_MODEL and identifier_field_name are evaluated at class definition time inside the conditional block. This means they are computed once when the module is imported. If the user model is swapped or changed dynamically, this will not be reflected. Consider moving this logic to a method or property that evaluates at runtime.
| schema = ManualSchema( | ||
| fields=[ | ||
| coreapi.Field( | ||
| name="username", | ||
| name=identifier_field_name, | ||
| required=True, | ||
| location='form', | ||
| schema=coreschema.String( | ||
| title="Username", | ||
| description="Valid username for authentication", | ||
| title=identifier_field_name.title(), | ||
| description=f"Valid {identifier_field_name} for authentication", | ||
| ), |
There was a problem hiding this comment.
The new dynamic schema generation based on USERNAME_FIELD lacks test coverage. Consider adding tests that verify the schema correctly reflects the USERNAME_FIELD from custom user models (e.g., when USERNAME_FIELD is 'email', the schema should show 'email' instead of 'username').
|
|
||
| def validate(self, attrs): | ||
| username = attrs.get('username') | ||
| identifier_value = attrs.get(self.identifier_fiend_name) |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| raise serializers.ValidationError(msg, code='authorization') | ||
| else: | ||
| msg = _('Must include "username" and "password".') | ||
| msg = _(f'Must include "{self.identifier_fiend_name}" and "password".') |
There was a problem hiding this comment.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
Summary
Django’s default user model uses
usernameandpasswordfor authentication. However, when a custom user model is defined withUSERNAME_FIELD = 'email', Django correctly uses email and password for authentication.The issue was that Django REST Framework's
obtain_auth_tokenendpoint (used for TokenAuthentication) still expectedusernameandpassword, even when a custom user model usedemailinstead ofusername.Fix
This update modifies the
ObtainAuthTokenview to dynamically useUSERNAME_FIELDinstead of assumingusername. Now, authentication works consistently, whether using the built-in User model or a custom one, with no inconsistencies.Changes Made:
ObtainAuthTokennow retrieves theUSERNAME_FIELDfrom the user model.USERNAME_FIELDandpassword, ensuring compatibility with both built-in and custom user models.Impact
USERNAME_FIELD = 'email') can now log in usingemailandpasswordinstead ofusernameandpassword.default Usermodel or acustom Usermodel whereUSERNAME_FIELD = 'username'can continue logging in usingusernameandpasswordas expected.